Refactor: eliminate quaternion derivative code duplication in u_dot_generalized_3dof#903
Merged
aZira371 merged 3 commits intoenh/3-dof-lateral-motion-improvementfrom Dec 3, 2025
Merged
Conversation
7 tasks
…eneralized_3dof Co-authored-by: aZira371 <99824864+aZira371@users.noreply.github.com>
Co-authored-by: aZira371 <99824864+aZira371@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Address feedback on 3-dof lateral motion improvement
Refactor: eliminate quaternion derivative code duplication in u_dot_generalized_3dof
Dec 3, 2025
aZira371
approved these changes
Dec 3, 2025
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## enh/3-dof-lateral-motion-improvement #903 +/- ##
========================================================================
- Coverage 80.91% 80.39% -0.52%
========================================================================
Files 107 106 -1
Lines 13486 13043 -443
========================================================================
- Hits 10912 10486 -426
+ Misses 2574 2557 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Gui-FernandesBR
approved these changes
Dec 3, 2025
Member
Gui-FernandesBR
left a comment
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Could separate the whole function into smaller methods... It makes code more readable and prevents refactors like this.
aZira371
merged commit 70c6b5f
into
enh/3-dof-lateral-motion-improvement
25 of 26 checks passed
Collaborator
@Gui-FernandesBR separating the whole u_generalized_3dof function ? maybe yes that can be done like can create a separate helper function which does the whole wc thing. I saw your comment after merging. Will add a TODO for it, can take this up later. But will keep the advice in mind. |
Gui-FernandesBR
pushed a commit
that referenced
this pull request
Dec 3, 2025
* ENH: addition of bella lui based 3 dof and 6 dof comparison notebook - ENH: a new notebook bella_lui_3dof_vs_6dof.ipynb which uses new implementations of weathercocking model on 3dof * ENH: addition of weathercocking model to flight.py - ENH: introduced new weathercock_coeff parameter in Flight.init (default: 1.0) - ENH: updated u_dot_generalized_3dof to compute quaternion derivatives proportional to misalignment with relative wind - ENH: angular velocity = weathercock_coeff * sin(misalignment_angle) * ENH: added tests for weathercocking to test_flight_3dof.py - ENH: unit tests added for weathercocking to check whether weathercock_coeff=0 results in fixed attitude (no quaternion change). - MNT: format and lint updates for new additions * DOC: updating 3 dof documentation and corresponding index.rst - DOC: added 3 dof and 6 dof comparison analysis to three_dof_simulation.rst - DOC: updated iusers/index.rst to build three_dof_simulation.rst - MNT: deleted the bella_lui_3dof_vs_6dof_comparison.ipynb as the doc now covers this section * MNT: corrections to test_flight_3dof.py - MNT: corrected doc string to represent correct orientation - MNT: improved tolerance of check on quaternion derivative which should be ideally very small when axes are aligned * MNT: docstring corrections to flight.py around new weathercocking implementation * BUG: correction of singularity bug during align on vectors in weathercocking - BUG: implemented a dot product check to ensure that singularity bug is avoided when rocket body and wind velocity are anti aligned to make rocket statically stable (in u_dot_generalized_3dof) - MNT: removed redundant double assignment of e0 and w0 vectors within u_dot_generalized_3dof - MNT: format correction to test_flight_3dof.py * MNT: updating location of 3 dof doc in users index.rst - MNT: 3 dof documentation only referred in users section/getting started - MNT: correction of docstring in flight.py - MNT: corrected unit_vector call when defining rotation axis in u_dot_generalized_3dof - MNT: docstring correction in test_flight_3dof.py - ENH: test coverage added for anti alignment case in weathercock model to test_flight_3dof.py * DOC: three_dof_simulation.rst update to add explanation of weather cocking coeff usage and value. * MNT: changed default value of weather_coeff in flight.py and added fixtures to test_flight_3dof.py * MNT: shifting test_flight_dof.py to integration tests. - MNT: fixed default weathercock_coeff value in flight.py to match 3 dof behaviour by default - MNT: corrected fixtures and docstrings in test_flight_3dof.py * MNT: docsrting update in test_flight_3dof.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * MNT: Update of docstring in test_flight_3dof.py - MNT: correction of docstring now that fixtures are used. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * DOC: Update of three_dof_simulation.rst Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tests/integration/simulation/test_flight_3dof.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Docstring Update tests/integration/simulation/test_flight_3dof.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * docstring Update tests/integration/simulation/test_flight_3dof.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * docstring Update docs/user/three_dof_simulation.rst Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Docstring Update rocketpy/simulation/flight.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * MNT: Docstring updates to various files related to 3 dof * MNT: unit vector edge case check in flight.py - MNT: perp_axis singularity value error implemented to handle edge case for perp_axis being parallel to body axis in weathercocking model of 3 dof. * DOC: Add note about motor file paths in 3-DOF comparison section (#902) * Initial plan * DOC: Add note about motor file paths in 3-DOF comparison section Co-authored-by: aZira371 <99824864+aZira371@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aZira371 <99824864+aZira371@users.noreply.github.com> * MNT: eliminate quaternion derivative code duplication in u_dot_generalized_3dof (#903) * Initial plan * Refactor: eliminate quaternion derivative code duplication in u_dot_generalized_3dof Co-authored-by: aZira371 <99824864+aZira371@users.noreply.github.com> * Improve comment clarity in weathercocking aligned case Co-authored-by: aZira371 <99824864+aZira371@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aZira371 <99824864+aZira371@users.noreply.github.com> * DOC: CHANGELOG.md update to reflect implementation of 3 dof lateral motion improvement --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request type
Checklist
pytest tests -m slow --runslow) have passed locallyCurrent behavior
The quaternion derivative calculation in
u_dot_generalized_3dofhad identical code blocks duplicated in two places (lines 1958-1962 and 1989-1993):New behavior
Refactored to compute
omega_bodyfirst in all branches, then calculate quaternion derivatives once:omega_bodybased on alignment state (normal, anti-aligned, or aligned/None)omega_body is not NoneThis reduces maintenance burden—formula updates only need to happen in one place.
Breaking change
Additional information
Addresses code review feedback on PR #883.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.